-
Notifications
You must be signed in to change notification settings - Fork 441
Added docs for query builder #1173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There is not an easy way of introducing a breaking change… It should be experimental and advised for some time before changing the whole api. Otherwise I guess we’re making it difficult for users. I think we can remove the old query and change the new one so it’s the default soon anyway |
I assume I can just reference QueryBuilder instead of ExperimentalQuery which I imagine won’t get removed in the future? |
If you look into the code ExperimentalQuery is just an alias: https://github.com/O365/python-o365/blob/master/O365/utils/__init__.py#L12 I will remove this alias in the future. Just use QueryBuilder like you said |
@RogerSelwyn thinking of setting the new Query as a default. What do you think? |
It;s fine for me, I already migrated. Just need to note a breaking change in the change log. The challenge will be where people don't have the version of O365 pinned. Alternatively, include a deprecation warning in the logs for a few months. |
The deprecation warning is in place since June 2025 (O365 version 2.1.4) |
I'll change the default Query to the new one for the moment but keep the old one as well |
@RogerSelwyn I've dropped setup.py completely (requirement.txt, etc.) and use the new pyproject.tml with uv: The problem is that I broke the github action to build the pages as it was using Do you mind helping me moving the github action to use uv and pyproject.toml? |
OK, so I will have to learn UV, probably time for me to anyway then I can do the same to my own projects. A next week task probably. Then I'm away for a week, 12-19. |
Probably didn't see it because I had migrated...
Good plan... |
@alejcas latest commit on here should build pages correctly when merged. I can move to separate PR if required. |
I noted these errors in my test build. I can fix them in this or a separate PR if needed.
|
First one requires the indentation removing from the fish line of the docs in the update_cells method. Second one should likely be this in sharepoint.rst:
|
Thank you very much @RogerSelwyn. I will merge this but the docs added here mention ExperimentalQuery and this no longer exist. It's QueryBuilder. |
Changed it |
Wow, I don't understand this. Seems like the indentation is correct (or as correct as in other methods in the same file) |
I think I'd have to go back and look in-depth at the Sphinx/ReadTheDocs documentation. I think in some places you just get away with doing it wrong. I don't think you are supposed to indent, but sometimes it allows it. |
Sphinx is great but very complicated for me. Also I hate rst format, would love markdown. Should l merge this then as it is? |
Give me 5 mins, I'll see if I can include those docs fixed in this PR |
You should be good to go now |
Thanks! |
Added docs for query builder based on old query helper docs. Two comments to make:
mailbox.new_query
. Now you must:Would it be feasible to add the new builder method to ApiComponent?